Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add verification key hash to the return type of ZkProgram compile #1292

Merged
merged 3 commits into from
Dec 5, 2023
Merged

Add verification key hash to the return type of ZkProgram compile #1292

merged 3 commits into from
Dec 5, 2023

Conversation

rpanic
Copy link
Contributor

@rpanic rpanic commented Dec 4, 2023

Previously, ZkProgram and Smartcontract had different return types for compile(). This PR fixes this by adding in the hash to ZkProgram's function

@mitschabaude
Copy link
Member

Fun fact: I have the same change in an open PR. So in this case I think it's fine to not add a changelog item, but if you still want to add it you can use the snippet in that PR: https://github.com/o1-labs/o1js/pull/1240/files#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR24

@rpanic
Copy link
Contributor Author

rpanic commented Dec 4, 2023

Thanks @mitschabaude, did that. Also aligned the formatting, maybe merging will become easier

mitschabaude
mitschabaude reviewed Dec 4, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@mitschabaude mitschabaude merged commit 03bb11f into o1-labs:main Dec 5, 2023
13 checks passed
@dfstio
Copy link

dfstio commented Dec 7, 2023

Should the verify() argument also be changed from string to VerificationKey?

@mitschabaude
Copy link
Member

Should the verify() argument also be changed from string to VerificationKey?

Great point. I'd like to see verify() handle both cases actually. String is the more logical input and VerificationKey is good for backwards compat

@mitschabaude
Copy link
Member

I went ahead and pushed it to the upcoming release PR #1294

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mitschabaude mitschabaude mitschabaude approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rpanic @mitschabaude @dfstio